Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make CockroachDB test be able to run on secure clusters. #724

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make CockroachDB test be able to run on secure clusters. #724

wants to merge 1 commit into from

Conversation

DarrylWong
Copy link

The setup script for cockroachdb tests previously passed an --insecure flag. This change replaces that with a more generic ARGS flag that gives more flexibility. The test can still run on insecure clusters if --insecure is passed or on secure clusters if certs-dir is passed.

PavloTytarchuk
PavloTytarchuk previously approved these changes Feb 5, 2024
View reviewed changes
Copy link
Contributor

@PavloTytarchuk PavloTytarchuk left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

@PavloTytarchuk PavloTytarchuk dismissed their stale review February 5, 2024 12:09

CI/CD tests are failing with these changes.

@PavloTytarchuk
Copy link
Contributor

Hello @DarrylWong! I've opened PR with your changes in our repo, to check if CI/CD builds will pass with your changes. Unfortunately build fails with: ERROR: cannot establish secure connection to insecure server.

If I remove --insecure from command: start-single-node --insecure in our docker-config, build starts to fail with: ERROR: cannot load certificates.

@DarrylWong
Make CockroachDB test be able to run on secure clusters.
ccce4c7 
The setup script for cockroachdb tests previously passed
an --insecure flag. This change replaces that with a more
generic ARGS flag that gives more flexibility. The test
can still run on insecure clusters if --insecure is passed
or on secure clusters if certs-dir is passed.
@DarrylWong
Copy link
Author

Hey Pavlo, thanks for taking a look at my PR!

I went with the easiest fix of passing the --insecure flag to the ./setup_db.sh:/setup_db.sh call in docker. This should retain the previous behavior of running the tests on insecure clusters for CI, but still be an improvement as anyone who wants to (namely cockroachdb's own tests) can run on secure mode.

Let me know if my reasoning makes sense! 😄

@liquibot liquibot removed the TypeBug label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KushnirykOleh KushnirykOleh Awaiting requested review from KushnirykOleh

@PavloTytarchuk PavloTytarchuk Awaiting requested review from PavloTytarchuk

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DarrylWong @PavloTytarchuk @KushnirykOleh @liquibot